feat(metadata-sidebar): Add handler for Autofill button #3700

jankowiakdawid · 2024-10-07T16:59:08Z

No description provided.

src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts

src/elements/content-sidebar/MetadataInstanceEditor.tsx

src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts

And bump metadata-editor package Wrap MetadataInstanceList and MetadataInstanceForm in Autofill provider Co-authored-by: Wiola (wpiesiak)

JChan106 · 2024-10-10T18:05:50Z

src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts

+        async (templateKey: string, fields: MetadataTemplateField[]) => {
+            const aiAPI = api.getIntelligenceAPI();
+
+            await wait(1000);


Why do we have a setTimeout here? We should have a constant for the delay time as well.

sorry, I was using it to test loading state, and forgot to delete

greg-in-a-box · 2024-10-10T18:02:14Z

src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts

+        async (templateKey: string, fields: MetadataTemplateField[]) => {
+            const aiAPI = api.getIntelligenceAPI();
+
+            await wait(1000);


what is the reason for this ?

sorry, I was using it to test loading state, and forgot to delete

greg-in-a-box · 2024-10-10T18:07:07Z

src/test-utils/testing-library.tsx

    <AutofillContextProvider
-        isAiSuggestionsFeatureEnabled={isAiSuggestionsFeatureEnabled}
        fetchSuggestions={fetchSuggestions}
+        isAiSuggestionsFeatureEnabled={isAiSuggestionsFeatureEnabled}
    >


after looking at this again and noticed we are passing props specifically for it, think we need to consider moving this out of the testing-library into the metadata-editor tests directly, all the tests in the repo will use this wrapper eventually, i cant imagine that provider being used that much to warrant being in the wrapper. the other providers where are more generic

Agree. AutofillContextProvider is needed only for MetadataInstanceEditor tests.
I did tried moving it out but it resulted in failing tests

TypeError: (0 , _reactIntl.useIntl) is not a function

Not quite sure what's happening. But I needed to move the whole wrapper component to MetadataInstanceEditor test.

tjuanitas · 2024-10-10T18:05:42Z

src/api/Intelligence.js

@@ -75,6 +75,7 @@ class Intelligence extends Base {
            suggestionsResponse = await this.xhr.post({
                url,
                data: request,
+                id: `file_${request.items[0].id}`,


i see we do this in the ask method as well but how important is this id field? if items has multiple files would this cause conflicts or unexpected behavior since we always look at the first index?

id is used internally by post to get auth token, it can by an array, but for this endpoint we only use one file. That is whybI take first element.

tjuanitas · 2024-10-10T18:14:11Z

src/elements/content-sidebar/MetadataInstanceEditor.tsx

@@ -22,9 +24,8 @@ export interface MetadataInstanceEditorProps {
    template: MetadataTemplateInstance;
 }

-const MetadataInstanceEditor: React.FC<MetadataInstanceEditorProps> = ({
+export const MetadataInstanceEditor: React.FC<MetadataInstanceEditorProps> = ({


nit: if we're exporting for unit tests, we usually add the export at bottom with something like:

export { MetadataInstanceEditor as MetadataInstanceEditorComponent };

example:

box-ui-elements/src/elements/content-sidebar/Sidebar.js

Lines 342 to 343 in b6b61bf

export { Sidebar as SidebarComponent };

export default flow([withCurrentUser, withFeatureConsumer, withRouter])(Sidebar);

We actually don't need the withApiWrapper so I will restore the old way we were exporting here.

tjuanitas · 2024-10-10T18:17:50Z

src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts

+        async (templateKey: string, fields: MetadataTemplateField[]) => {
+            const aiAPI = api.getIntelligenceAPI();
+
+            await wait(1000);


what's the reasoning for this? are we adding an artificial loading time?

sorry, I was using it to test loading state, and forgot to delete

tjuanitas

comment but approved. please address in follow up if not fixed in this PR

tjuanitas · 2024-10-10T21:09:29Z

src/test-utils/testing-library.tsx

 );

-type RenderConnectedOptions = Omit<RenderOptions, 'wrapper'> & {
+type RenderConnectedOptions = RenderOptions & {
+    // Omit<RenderOptions, 'wrapper'> & {


uncomment or remove?

jankowiakdawid requested review from a team as code owners October 7, 2024 16:59

jankowiakdawid commented Oct 7, 2024

View reviewed changes

src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts Outdated Show resolved Hide resolved

wpiesiak reviewed Oct 8, 2024

View reviewed changes

src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts Outdated Show resolved Hide resolved

wpiesiak reviewed Oct 8, 2024

View reviewed changes

src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts Outdated Show resolved Hide resolved

wpiesiak reviewed Oct 8, 2024

View reviewed changes

src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts Show resolved Hide resolved

wpiesiak reviewed Oct 8, 2024

View reviewed changes

src/elements/content-sidebar/MetadataInstanceEditor.tsx Outdated Show resolved Hide resolved

jankowiakdawid force-pushed the handle-autofill branch 2 times, most recently from 0bed1bb to 21d879a Compare October 9, 2024 14:50

JChan106 reviewed Oct 9, 2024

View reviewed changes

src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts Outdated Show resolved Hide resolved

jankowiakdawid added 3 commits October 10, 2024 19:41

feat(metadata-sidebar): Add handler for Autofill button

d88af63

feat(metadata-sidebar): Handle Error by rethrowing them to host app

1a39b96

feat(metadata-sidebar): Add error handling

6243548

And bump metadata-editor package Wrap MetadataInstanceList and MetadataInstanceForm in Autofill provider Co-authored-by: Wiola (wpiesiak)

jankowiakdawid force-pushed the handle-autofill branch from 3f6575d to 6243548 Compare October 10, 2024 17:51

JChan106 reviewed Oct 10, 2024

View reviewed changes

greg-in-a-box requested changes Oct 10, 2024

View reviewed changes

tjuanitas reviewed Oct 10, 2024

View reviewed changes

jankowiakdawid added 2 commits October 10, 2024 22:09

feat(metadata-sidebar): Remove unnecessary wait function

2bb0fc1

feat(metadata-sidebar): Restore old export in MetadataInstanceEditor

c6aba6d

jankowiakdawid added the ready-to-merge label Oct 10, 2024

tjuanitas previously approved these changes Oct 10, 2024

View reviewed changes

feat(metadata-sidebar): Move AutofillContextProvider out of test-utils

3d8ba70

jankowiakdawid dismissed tjuanitas’s stale review via 3d8ba70 October 10, 2024 21:16

jankowiakdawid force-pushed the handle-autofill branch from b17dc53 to 3d8ba70 Compare October 10, 2024 21:16

greg-in-a-box approved these changes Oct 10, 2024

View reviewed changes

tjuanitas approved these changes Oct 10, 2024

View reviewed changes

JChan106 approved these changes Oct 10, 2024

View reviewed changes

tjuanitas approved these changes Oct 10, 2024

View reviewed changes

Merge branch 'master' into handle-autofill

f449840

mergify bot merged commit 95735e0 into box:master Oct 10, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(metadata-sidebar): Add handler for Autofill button #3700

feat(metadata-sidebar): Add handler for Autofill button #3700

jankowiakdawid commented Oct 7, 2024

JChan106 Oct 10, 2024

jankowiakdawid Oct 10, 2024

greg-in-a-box Oct 10, 2024

jankowiakdawid Oct 10, 2024

greg-in-a-box Oct 10, 2024

jankowiakdawid Oct 10, 2024

tjuanitas Oct 10, 2024 •

edited

Loading

jankowiakdawid Oct 10, 2024

tjuanitas Oct 10, 2024

jankowiakdawid Oct 10, 2024

tjuanitas Oct 10, 2024

jankowiakdawid Oct 10, 2024

tjuanitas left a comment

tjuanitas Oct 10, 2024

	export { Sidebar as SidebarComponent };
	export default flow([withCurrentUser, withFeatureConsumer, withRouter])(Sidebar);

feat(metadata-sidebar): Add handler for Autofill button #3700

feat(metadata-sidebar): Add handler for Autofill button #3700

Conversation

jankowiakdawid commented Oct 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjuanitas Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjuanitas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjuanitas Oct 10, 2024 •

edited

Loading